Skip to content

chore: better error handling for commands - #63

Merged
k3llymariee merged 4 commits into
mainfrom
kelly/sc-237645/better-error-handling-for-commands
Mar 25, 2024
Merged

chore: better error handling for commands#63
k3llymariee merged 4 commits into
mainfrom
kelly/sc-237645/better-error-handling-for-commands

Conversation

@k3llymariee

@k3llymariee k3llymariee commented Mar 25, 2024

Copy link
Copy Markdown
Contributor

Adds a new error type and error method to handle errors returned by the LD go client, displaying the json response body.

Structural errors like missing flag or invalid commands will still show an error and return usage, logical errors like like invalid access token or project key will show the error but not usage.

image

@shortcut-integration

Copy link
Copy Markdown

This pull request has been linked to Shortcut Story #237645: have better error handling for commands.

Comment thread internal/errors/errors.go
Comment on lines +45 to +49
var ldErr *ldapi.GenericOpenAPIError
ok := errors.As(err, &ldErr)
if ok {
return nil, NewErrorWrapped(string(ldErr.Body()), ldErr)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API should always return errors in a common format per our documentation, so we should be able to rely on the ldapi.GenericOpenAPIError to get the json message body out.

@k3llymariee
k3llymariee marked this pull request as ready for review March 25, 2024 20:00
Comment thread internal/errors/errors.go Outdated
})
}

func NewApiError(err error) ([]byte, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you have this return only the error? It's confusing why it also returns a nil byte array.

And could you rename it to NewAPIError?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops yeah that was a goland refactor thing - and done!

@k3llymariee
k3llymariee merged commit b837578 into main Mar 25, 2024
@k3llymariee
k3llymariee deleted the kelly/sc-237645/better-error-handling-for-commands branch March 25, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants